Skip to content

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Sep 25, 2025

For WOOMOB-869

Just one review is required.

Description

This PR refactors the POS tab logic to separate visibility and eligibility concerns, following the architecture discussion in PR #16153. It also removes the pointOfSaleAsATabi2 feature flag, which contributes to ~1000 deletion diffs.

Changes Made

  • Refactored POSTabEligibilityChecker to focus purely on eligibility check based on plugin version and feature switch eligibility, and POSTabVisibilityChecker now handles the visibility logic including CIAB checks
  • Removed pointOfSaleAsATabi2 feature flag, and LegacyPOSTabEligibilityChecker + LegacyPOSTabEligibilityCheckerTests that were used when the feature flag was disabled

Architecture Improvements

  • Clear separation of concerns: Visibility checker handles country/currency/CIAB, eligibility checker handles plugin/versions
  • Reduced coupling: Eligibility checker no longer depends on full Site model and can be set immediately after switching stores when the site ID becomes available before the full Site
  • Maintainability: Each checker has a single, well-defined responsibility

Steps to reproduce

  1. Build and run the app on an iPad
  2. Log in to a WPCOM account connected to more than one store eligible for POS (US/GB with proper currency)
  3. Tap on the POS tab --> the products should be loaded for the connected store
  4. Switch to another store and tap on the POS tab immediately --> the products should be loaded for the connected store

Testing information

I tested the above on iPad, and on iPhone to ensure that POS tab wasn't shown.


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@dangermattic
Copy link
Collaborator

dangermattic commented Sep 25, 2025

2 Warnings
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@jaclync jaclync added type: technical debt Represents or solves tech debt of the project. feature: POS labels Sep 25, 2025
@jaclync jaclync added this to the 23.4 milestone Sep 25, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 25, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16173-d610283
Version23.3
Bundle IDcom.automattic.alpha.woocommerce
Commitd610283
Installation URL3rog3c4kvr91g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync jaclync changed the title Refactor POS Tab: Separate Visibility and Eligibility Logic Refactor POS tab: separate visibility and eligibility logic while removing POS as a tab i2 feature flag Sep 25, 2025
@iamgabrielma iamgabrielma self-assigned this Sep 29, 2025
Copy link
Contributor

@iamgabrielma iamgabrielma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests well, switching between an US and UK store loads the site products correctly. I just saw the POS tab glitching for a moment when I first switched stores (POS tab becoming invisible) but was restored immediately, which I guess happened while checking the requirements and waiting for network response. All good!

Tested in iPad Pro 12.9 inch - iOS 26.0

return .ineligible(reason: .unsupportedInCIABSites)
}

guard #available(iOS 17.0, *) else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We can remove this availability check as well and the unsupportedIOSVersion case as ineligible reason

)
}

@available(iOS 17.0, *)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We can remove this availability check as well

stockStatusKey: ""))
}

@available(iOS 17.0, *)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We can remove this availability check as well

…e-flag-with-refactoring

# Conflicts:
#	Modules/Sources/Experiments/FeatureFlag.swift
@jaclync
Copy link
Contributor Author

jaclync commented Sep 30, 2025

Tests well, switching between an US and UK store loads the site products correctly. I just saw the POS tab glitching for a moment when I first switched stores (POS tab becoming invisible) but was restored immediately, which I guess happened while checking the requirements and waiting for network response. All good!

I believe this is due to the recent CIAB change 8b32ea3 that delays the tab visibility check until the full site becomes available, which takes place a bit after the site ID becomes available, when we used to check the tab visibility. This delay causes the tab visibility "glitching" (changing back and forth). I'll see if I can fix this separately for Kiwi's review.

…e-flag-with-refactoring

# Conflicts:
#	WooCommerce/WooCommerce.xcodeproj/project.pbxproj
@jaclync
Copy link
Contributor Author

jaclync commented Sep 30, 2025

Update from #16173 (comment):

I'll see if I can fix this separately for Kiwi's review.

As the latest visibility check requires the CIAB check from the full site

guard siteCIABEligibilityChecker.isFeatureSupported(.pointOfSale, for: site) else {
return false
}

I'm not sure if there's a good way to perform this check before the full site becomes available. A workaround is to skip the CIAB check when the site ID first becomes available, then include the CIAB check when the full site becomes available later. However, this could also result in a glitch if the visibility changes in the second check. It feels like a tradeoff from the CIAB change that we are accepting. Once the visibility check is performed from the full site, it should be cached and used for initial visibility next time when the full site is available. Please feel free to share any ideas for making the POS visibility available earlier like before the CIAB change @itsmeichigo @RafaelKayumov.

@jaclync jaclync enabled auto-merge September 30, 2025 06:14
@jaclync jaclync merged commit 6a1c368 into trunk Sep 30, 2025
13 checks passed
@jaclync jaclync deleted the backlog/WOOMOB-869-remove-pos-tab-i2-feature-flag-with-refactoring branch September 30, 2025 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: technical debt Represents or solves tech debt of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants